Conversation
|
@FetoiuCatalin please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
This PR addresses gaps in WSL mirrored networking route mirroring, specifically improving handling of default on-link routes (no next hop) and adjusting mirrored route metrics to better match Linux behavior.
Changes:
- Add a Windows NetworkTests coverage case for adding default on-link routes in both IPv4 and IPv6.
- Update mirrored route metric mapping to avoid emitting metric 0.
- Extend Linux netlink route programming to support default on-link routes by sending a default route without
RTA_GATEWAY.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/windows/NetworkTests.cpp | Adds a new test validating default on-link routes for IPv4/IPv6. |
| src/windows/service/exe/WslMirroredNetworking.cpp | Adjusts route metric handling to remap metric 0 to 1. |
| src/linux/netlinkutil/RoutingTable.h | Declares a new helper for default on-link route netlink messages. |
| src/linux/netlinkutil/RoutingTable.cpp | Implements default on-link default-route netlink messaging and updates default-route logging. |
f8b1912 to
ea607a8
Compare
ea607a8 to
9c60a6d
Compare
| route.to.has_value() ? route.to.value().Addr().c_str() : "[empty]", | ||
| route.via.has_value() ? route.via.value().Addr().c_str() : "[empty]", | ||
| route.to.value().Addr().c_str(), | ||
| "[empty]", |
There was a problem hiding this comment.
route.via might have a value, right?
There was a problem hiding this comment.
it's guaranteed in all possible paths that calling ModifyLinkLocalRouteImpl can never have a route.via?
There was a problem hiding this comment.
if so, we should fail the request if route.via.has_value().
this would be silently not doing what the caller expected if route.via.has_value()
| "SendMessage Route (to {}, via {}), operation ({}), netLinkflags ({})", | ||
| route.to.has_value() ? route.to.value().Addr().c_str() : "[empty]", | ||
| route.via.has_value() ? route.via.value().Addr().c_str() : "[empty]", | ||
| "[empty]", |
There was a problem hiding this comment.
is it guaranteed that route.to is always empty?
if so, we should fail the request if route.to.has_value().
this would be silently not doing what the caller expected if route.to.has_value()
Summary of the Pull Request
Testing mirrored mode with Proton VPN exposed some issues with route mirroring:
Validation Steps Performed
Testing with ProtonVPN and confirming v6 traffic is correctly routed over the mirrored interface that corresponds to the VPN
Running all NetworkTests
Added new tests that verify adding default onlink routes works as expected for both IPv4 and IPv6